Support u64 vertex ids in the bftree provider by JordanMaples · Pull Request #1216 · microsoft/DiskANN

JordanMaples · 2026-06-30T21:28:51Z

What

Make the bf-tree provider generic over its vertex-id type so it can index past
the u32::MAX (~4.3B) ceiling, while keeping u32 as the default (no memory or
storage cost for existing datasets). Also add defensive hardening, wide-id test
coverage, and a benchmark-layer path to validate the u64 id space against
real-world datasets.

New id.rs — BfTreeId trait with u32 / u64 impls, from_index /
try_from_index, and validate_id_capacity.
provider.rs — BfTreeProvider<T, Q = QuantVectorProvider, I = u32>.
The vertex id type threads through construction, save, and load. The persisted
SavedParams records id_width; load validates it against the requested type
so a u32 index can't be silently reopened as u64 (or vice versa).
neighbors.rs / vectors.rs — neighbor cells and starting-point seeding
are generic over I, sized via size_of::().

Why

Larger-than-memory, billion-scale datasets need vertex ids wider than u32.
Defaulting I = u32 keeps the common case unchanged; opting into u64 is a
single type parameter at the call site.

Hardening

Defensive coverage for the wide-id path:

Capacity guard rejecting id counts that overflow the chosen width.
Save/load rejects an id_width mismatch instead of misreading data.
Tests for high-bit (> u32::MAX) ids and their low-bit collisions, plus a
u64 save/load round-trip.

Wide-id tests + 32-bit compile guard

The earlier u64-typed tests only used values in 0..N, which still fit in
u32 and would pass even under silent 32-bit truncation. Added tests that
actually exceed u32::MAX:

test_quantized_index_search_u64_high_ids — end-to-end quantized insert +
search with ids above u32::MAX, using a sparse max_points so no
billion-scale allocation is needed.
test_u64_id_recall_parity_on_transposed_dataset — builds a random dataset
twice (dense u32 identity vs an order-preserving u64 offset above
u32::MAX) and asserts identical per-query neighbor sets plus a recall floor
against brute-force ground truth.

Added BfTreeId::INDEX_CONVERSION_LOSSLESS, overridden for u64 to assert a
64-bit usize and referenced from the monomorphized validate_id_capacity.
This turns as_index truncation into a compile error only for u64 providers
on 32-bit targets, leaving the default u32 path (even on 32-bit) unaffected.

Real-world `u64` validation (benchmark layer)

Real datasets ship as a dense 0..N block whose ids always fit in u32, so
they never exercise the u64 paths, and reaching an id > u32::MAX
positionally would need billions of rows. Added an opt-in, validation-only
benchmark path that transposes a dataset's dense u32 row ids into the
u64-only range (base > u32::MAX) without touching production code:

diskann-benchmark-core: an Offset ToId<u64> mapper (base + i, checked
add), documented as validation-only.
diskann-benchmark: a single_or_multi_insert_offset helper, a
U64IdValidation { offset } JSON config on BfTreeFullPrecisionBuild
(#[serde(default)], non-breaking), a widened bftree_parameters_offset, and
a dedicated run_u64_id_validation orchestration that builds a
BfTreeProvider<T, NoStore, u64>, inserts with the offset, and widens
ground-truth ids via Matrix::map to keep recall aligned. This reuses the
already-generic provider/search/recall stacks rather than relaxing the shared
u32 build pipeline.
Example config + an end-to-end integration test asserting the transposed
u64 run reproduces the identity-u32 recall baseline.

Validated on the 100K Wikipedia/Cohere dataset (offset 2^32, so every vertex
id lives above u32::MAX): recall@100 0.9536 (u32) vs 0.9537 (u64) — parity
confirmed (the sub-1e-4 delta is nondeterministic 8-thread multi-insert batch
ordering, not an id-path difference).

Deferred: the streaming benchmark is not yet transposable — its shared
executor is u32-pinned (the TagSlotManager slot allocator, the
ManagedStream trait, the Managed translation layer, and BfTreeStream all
hardcode u32), so a transposed stream run needs a broader genericization of
that shared infrastructure. Tracked as follow-up.

Notes

Each commit builds and lints clean in isolation (clean bisect).
Open design question: bf-tree's I is an internal id. The inmem2
direction uses InternalId = u32 (hardcoded) + generic external id. Worth
aligning on whether bftree needs wide internal ids or should follow the same
internal-u32 / external-generic split before merge.

Copilot

Pull request overview

This PR generalizes the diskann-bftree provider to support u64 vertex IDs (while keeping u32 as the default), enabling bf-tree indexes to exceed the u32::MAX vertex ceiling. It threads a new BfTreeId abstraction through the provider, neighbor storage, vector starting-point logic, and save/load metadata, and adds tests that exercise the wide-id path end-to-end.

Changes:

Introduces BfTreeId (u32/u64) and adds capacity validation to prevent silent truncation when minting IDs from indices.
Makes BfTreeProvider and related accessors/strategies generic over vertex-id type I, including neighbor record sizing via size_of::().
Persists id_width in SavedParams and validates width/capacity on load; adds tests for u64 high-bit IDs and u64 save/load round-trips.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
diskann-bftree/src/id.rs	Adds `BfTreeId` trait + capacity validation helper for index-derived IDs.
diskann-bftree/src/lib.rs	Exposes the new `id` module and re-exports `BfTreeId`.
diskann-bftree/src/vectors.rs	Makes starting-point ID generation generic over `BfTreeId`.
diskann-bftree/src/neighbors.rs	Sizes bf-tree neighbor records based on `I` width; adds `u64` ID collision/round-trip test.
diskann-bftree/src/provider.rs	Makes `BfTreeProvider` generic over `I`; adds save/load `id_width` validation and `u64` path tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov-commenter · 2026-06-30T21:44:07Z

Codecov Report

❌ Patch coverage is 92.88390% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.68%. Comparing base (bed3ef8) to head (ab2a1b0).

Files with missing lines	Patch %	Lines
diskann-bftree/src/provider.rs	94.63%	24 Missing ⚠️
diskann-bftree/src/id.rs	75.00%	13 Missing ⚠️
diskann-bftree/src/vectors.rs	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1216      +/-   ##
==========================================
+ Coverage   90.67%   90.68%   +0.01%     
==========================================
  Files         514      515       +1     
  Lines       98764    99220     +456     
==========================================
+ Hits        89554    89981     +427     
- Misses       9210     9239      +29

Flag	Coverage Δ
miri	`90.68% <92.88%> (+0.01%)`	⬆️
unittests	`90.36% <92.88%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
diskann-bftree/src/lib.rs	`59.09% <ø> (ø)`
diskann-bftree/src/neighbors.rs	`97.42% <100.00%> (+0.11%)`	⬆️
diskann-bftree/src/vectors.rs	`89.58% <66.66%> (ø)`
diskann-bftree/src/id.rs	`75.00% <75.00%> (ø)`
diskann-bftree/src/provider.rs	`91.69% <94.63%> (+1.21%)`	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

harsha-simhadri · 2026-06-30T22:01:13Z

Does u64 bloat adjacency list, and if so, is there a chance we can bring in a delta compression ?

JordanMaples · 2026-07-01T16:57:42Z

Does u64 bloat adjacency list, and if so, is there a chance we can bring in a delta compression ?

@harsha-simhadri yes, in u64 mode it doubles the adjacency-list storage (neighbor ids are stored inline), but that's explicit opt-in and there are no u64 consumers yet. I can tackle delta compression as a follow-up. Neighbor order doesn't affect search correctness, so we can sort ids and delta+varint them, which recovers essentially all of the u64 overhead.

harsha-simhadri · 2026-07-01T17:44:20Z

Does u64 bloat adjacency list, and if so, is there a chance we can bring in a delta compression ?

@harsha-simhadri yes, in u64 mode it doubles the adjacency-list storage (neighbor ids are stored inline), but that's explicit opt-in and there are no u64 consumers yet. I can tackle delta compression as a follow-up. Neighbor order doesn't affect search correctness, so we can sort ids and delta+varint them, which recovers essentially all of the u64 overhead.

thanks, could you create an issue for future follow up. thanks

…w guards Address review feedback on the u64 bftree provider (PR #1216): - Replace the `IntoUsize` supertrait bound with a first-class `BfTreeId::as_index` method, so the id abstraction owns its index arithmetic instead of leaning on a generic conversion. - Bake dense id iteration into `BfTreeId::id_range`, returning a concrete `IdRange` that maps `from_index` inside `next()`. This keeps the conversion a zero-sized, inlinable fn item rather than the stored `fn(usize) -> I` pointer that `Range::map` folded in (which defeated inlining and forced an indirect call per neighbor id), while preserving the exact-size and double-ended properties of the range. - Guard the `max_points + num_start_points` and `max_points + frozen_points` sums with `checked_add`, returning an `ANNError` instead of risking a `usize` overflow. Kept the `VectorId` supertrait: the core `HasId::Id` and `DataProvider::InternalId` associated types both require it, so dropping it would only force `I: VectorId` to be restated at every impl site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

harsha-simhadri · 2026-07-13T20:11:54Z

Coud you please link the follow up issues on delta encoding for u64 in bf-tree
Share the testing done for u64 id type, and the results.

JordanMaples · 2026-07-13T20:52:07Z

@harsha-simhadri

Coud you please link the follow up issues on delta encoding for u64 in bf-tree

The follow up issue is already linked above your comment.
#1234

JordanMaples · 2026-07-13T21:03:12Z

@harsha-simhadri
As far as tests for u64 go, I added 3 unit tests in this PR but haven't run against a real-word dataset.

…w guards Address review feedback on the u64 bftree provider (PR #1216): - Replace the `IntoUsize` supertrait bound with a first-class `BfTreeId::as_index` method, so the id abstraction owns its index arithmetic instead of leaning on a generic conversion. - Bake dense id iteration into `BfTreeId::id_range`, returning a concrete `IdRange` that maps `from_index` inside `next()`. This keeps the conversion a zero-sized, inlinable fn item rather than the stored `fn(usize) -> I` pointer that `Range::map` folded in (which defeated inlining and forced an indirect call per neighbor id), while preserving the exact-size and double-ended properties of the range. - Guard the `max_points + num_start_points` and `max_points + frozen_points` sums with `checked_add`, returning an `ANNError` instead of risking a `usize` overflow. Kept the `VectorId` supertrait: the core `HasId::Id` and `DataProvider::InternalId` associated types both require it, so dropping it would only force `I: VectorId` to be restated at every impl site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

diskann-bftree/src/provider.rs:1666

The load-time id-width mismatch error message is grammatically awkward for 8-byte ids ("a 8-byte") and could be clearer by avoiding the article and repeating "vertex ids" for both widths.

        return Err(ANNError::log_index_error(format!(
            "index was built with {}-byte vertex ids but is being loaded with a {expected}-byte \
             id type; load it with the matching id type",
            saved_params.id_width,
        )));

diskann-bftree/src/provider.rs:3655

This test constructs max_points as u32::MAX as usize + 2, which will overflow usize on 32-bit targets (wrapping in release or panicking in debug) and makes the test invalid there. Gate the test to 64-bit targets (where the intended capacity value is representable).

    async fn test_new_rejects_capacity_exceeding_id_type() {

…w guards Address review feedback on the u64 bftree provider (PR #1216): - Replace the `IntoUsize` supertrait bound with a first-class `BfTreeId::as_index` method, so the id abstraction owns its index arithmetic instead of leaning on a generic conversion. - Bake dense id iteration into `BfTreeId::id_range`, returning a concrete `IdRange` that maps `from_index` inside `next()`. This keeps the conversion a zero-sized, inlinable fn item rather than the stored `fn(usize) -> I` pointer that `Range::map` folded in (which defeated inlining and forced an indirect call per neighbor id), while preserving the exact-size and double-ended properties of the range. - Guard the `max_points + num_start_points` and `max_points + frozen_points` sums with `checked_add`, returning an `ANNError` instead of risking a `usize` overflow. Kept the `VectorId` supertrait: the core `HasId::Id` and `DataProvider::InternalId` associated types both require it, so dropping it would only force `I: VectorId` to be restated at every impl site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The provider hardcoded `u32` vertex ids, capping any bf-tree-backed index at ~4.29B vectors. That ceiling is at odds with the crate's purpose of supporting larger-than-memory, billion-scale-and-beyond datasets. Introduce a `BfTreeId` trait (`VectorId + IntoUsize` plus index-construction) implemented for `u32` and `u64`, and thread a new id type parameter `I` through `BfTreeProvider<T, Q, I = u32>` and all of its accessors, strategies, and save/load impls. The default `I = u32` keeps every existing caller (benchmark harness, tests) compiling and behaving identically, while `u64` is now available opt-in for indexes that exceed the 32-bit range. `iter()`/`IntoIterator` now map `0..total` through `I::from_index` instead of relying on `Range<u32>` (a generic `Range` would require the unstable `Step` trait). `VectorProvider::starting_points` is likewise generic over the id type. The on-disk neighbor format follows the id width and is self- consistent on load (the width is not persisted, mirroring the metric). A new `test_quantized_index_search_u64_ids` builds and searches a `BfTreeProvider<_, _, u64>` index end-to-end to prove the u64 path is functional, not merely compilable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Reinforce the newly-reachable u64 id path surfaced by review: - neighbors: size record key/value buffers by size_of::() instead of hardcoded size_of::<u32>(), so u64 providers reserve correct record sizes. - id: add validate_id_capacity to reject vertex counts that overflow the id type, and guard new_empty with it. - provider: persist id_width in SavedParams (back-compat default of 4) and validate it on load, so a u64 index can't be silently loaded as u32. - tests: add u64 high-bit id coverage (ids/neighbors > u32::MAX, low-bit collision check) plus a u64 save/load round-trip, id-width mismatch rejection, and capacity-guard rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…w guards Address review feedback on the u64 bftree provider (PR #1216): - Replace the `IntoUsize` supertrait bound with a first-class `BfTreeId::as_index` method, so the id abstraction owns its index arithmetic instead of leaning on a generic conversion. - Bake dense id iteration into `BfTreeId::id_range`, returning a concrete `IdRange` that maps `from_index` inside `next()`. This keeps the conversion a zero-sized, inlinable fn item rather than the stored `fn(usize) -> I` pointer that `Range::map` folded in (which defeated inlining and forced an indirect call per neighbor id), while preserving the exact-size and double-ended properties of the range. - Guard the `max_points + num_start_points` and `max_points + frozen_points` sums with `checked_add`, returning an `ANNError` instead of risking a `usize` overflow. Kept the `VectorId` supertrait: the core `HasId::Id` and `DataProvider::InternalId` associated types both require it, so dropping it would only force `I: VectorId` to be restated at every impl site. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add two tests exercising bf-tree vertex ids beyond u32::MAX: - test_quantized_index_search_u64_high_ids: end-to-end quantized insert + search with ids above u32::MAX, using a sparse max_points so no billion-scale allocation is needed. - test_u64_id_recall_parity_on_transposed_dataset: builds a random dataset twice (dense u32 identity vs order-preserving u64 offset above u32::MAX) and asserts identical neighbor sets per query plus a recall floor against brute-force ground truth. Existing u64 tests only used the u64 type with values in 0..N, which still fit in u32 and would pass even under silent 32-bit truncation. Also add a BfTreeId::INDEX_CONVERSION_LOSSLESS associated const, overridden for u64 to assert a 64-bit usize and referenced from the monomorphized validate_id_capacity. This turns the as_index truncation into a compile error only for u64 providers on 32-bit targets, leaving the default u32 path (even on 32-bit) unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d3795cf4-fd2c-46db-ac06-fc4a360392ba

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

diskann-bftree/src/vectors.rs:105

The new error message for start-point id overflow no longer indicates which id type failed or what limit was exceeded, which makes debugging harder (especially now that ids can be u32 or u64). Include the concrete id type and its width in the message.

            .map(|i| {
                I::try_from_index(i).ok_or_else(|| {
                    ANNError::message(lazy_format!(
                        move,
                        "start point id {i} exceeds the id type's maximum"

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

JordanMaples requested review from a team and Copilot June 30, 2026 21:28

Copilot started reviewing on behalf of JordanMaples June 30, 2026 21:39 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

Comment thread diskann-bftree/src/provider.rs Outdated

Comment thread diskann-bftree/src/provider.rs Outdated

hildebrandmw reviewed Jul 1, 2026

View reviewed changes

Comment thread diskann-bftree/src/provider.rs Outdated

Comment thread diskann-bftree/src/id.rs Outdated

JordanMaples mentioned this pull request Jul 7, 2026

Delta Compression for U64 based ids #1234

Open

JordanMaples enabled auto-merge (squash) July 15, 2026 23:11

JordanMaples force-pushed the jordanmaples/bftree_u64_ids branch from 3583acb to 5d4d540 Compare July 15, 2026 23:18

Copilot AI review requested due to automatic review settings July 27, 2026 19:00

JordanMaples force-pushed the jordanmaples/bftree_u64_ids branch from 5d4d540 to b07f493 Compare July 27, 2026 19:00

Copilot started reviewing on behalf of JordanMaples July 27, 2026 19:01 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread diskann-benchmark/src/index/build.rs Outdated

Copilot AI review requested due to automatic review settings July 27, 2026 21:00

Copilot started reviewing on behalf of JordanMaples July 27, 2026 21:00 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Comment thread diskann-benchmark/src/index/bftree/full_precision.rs Outdated

Copilot AI review requested due to automatic review settings July 30, 2026 15:58

JordanMaples force-pushed the jordanmaples/bftree_u64_ids branch from 01ea329 to 9217da1 Compare July 30, 2026 15:58

Copilot started reviewing on behalf of JordanMaples July 30, 2026 15:58 View session

Copilot AI reviewed Jul 30, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 30, 2026 17:12

JordanMaples force-pushed the jordanmaples/bftree_u64_ids branch from 9217da1 to ebf3dbc Compare July 30, 2026 17:12

Copilot started reviewing on behalf of JordanMaples July 30, 2026 17:14 View session

Jordan Maples and others added 4 commits July 30, 2026 10:17

JordanMaples force-pushed the jordanmaples/bftree_u64_ids branch from ebf3dbc to ab2a1b0 Compare July 30, 2026 17:18

Copilot AI reviewed Jul 30, 2026

View reviewed changes

Comment thread diskann-bftree/src/provider.rs

Copilot AI review requested due to automatic review settings July 30, 2026 17:21

Copilot started reviewing on behalf of JordanMaples July 30, 2026 17:22 View session

Copilot AI reviewed Jul 30, 2026

View reviewed changes

harsha-simhadri approved these changes Jul 30, 2026

View reviewed changes

hildebrandmw approved these changes Jul 30, 2026

View reviewed changes

JordanMaples merged commit 17dc61b into main Jul 30, 2026
28 checks passed

JordanMaples deleted the jordanmaples/bftree_u64_ids branch July 30, 2026 19:43

Uh oh!

Conversation

JordanMaples commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Hardening

Wide-id tests + 32-bit compile guard

Real-world u64 validation (benchmark layer)

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

harsha-simhadri commented Jun 30, 2026

Uh oh!

JordanMaples commented Jul 1, 2026

Uh oh!

harsha-simhadri commented Jul 1, 2026

Uh oh!

Uh oh!

Uh oh!

harsha-simhadri commented Jul 13, 2026

Uh oh!

JordanMaples commented Jul 13, 2026

Uh oh!

JordanMaples commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JordanMaples commented Jun 30, 2026 •

edited

Loading

Real-world `u64` validation (benchmark layer)

codecov-commenter commented Jun 30, 2026 •

edited

Loading